Free threaded resources on Android #2761
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alasram
requested review from
TimSylvester,
mwilsnd,
nyurik,
sjg-wdw and
adrian-cojocaru
Bloaty Results (iOS) 🐋Compared to main Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results-ios/pr-2761-compared-to-main.txt |
Bloaty Results 🐋Compared to main Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-2761-compared-to-main.txtCompared to d387090 (legacy) Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-2761-compared-to-legacy.txt |
|
Benchmark Results ⚡ Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/benchmark-results/pr-2761-compared-to-main.txt |
louwers
reviewed
Aug 24, 2024
louwers
added
cpp-core
OpenGL
alasram
force-pushed
the
alasram/free_threaded_resources
branch
3 times, most recently
from
c676779
to
aa00054
Compare
alasram
force-pushed
the
alasram/free_threaded_resources
branch
4 times, most recently
from
bb58491
to
8c1f7c7
Compare
alasram
force-pushed
the
alasram/free_threaded_resources
branch
3 times, most recently
from
f314bdf
to
0d64d6c
Compare
|
Just tagging @mwilsnd and @alexcristici to take a look at this. |
|
I will be making this opt-in because the performance is variable depending on vendors:
|
alasram
force-pushed
the
alasram/free_threaded_resources
branch
from
0d64d6c
to
22e5b23
Compare
louwers
reviewed
Sep 18, 2024
There was a problem hiding this comment.
Is the added complexity worth it if it is opt-in?
Probably a tiny fraction of users has enough OpenGL knowledge and control over their distribution for this to be useful as a feature.
If the performance gains are significant, can we make an informed choice for the user automatically?
alasram
force-pushed
the
alasram/free_threaded_resources
branch
4 times, most recently
from
20ba5d1
to
9838211
Compare
alasram
force-pushed
the
alasram/free_threaded_resources
branch
from
9838211
to
5185804
Compare
|
I changed this to always on by default on emulator and any device if GLES 3 is supported otherwise a GLES 2 context is requested and multi threading is disabled. |
alasram
force-pushed
the
alasram/free_threaded_resources
branch
from
5185804
to
5865533
Compare
…ze GL contexts but only use fences with GLES 3.
alasram
force-pushed
the
alasram/free_threaded_resources
branch
from
5865533
to
8e0b4ed
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When tiles are streamed, GPU resources are uploaded to the GPU synchronously in the render thread. A quick change in Zoom or location can block the render thread due to very slow uploads: Many uploads are serialized delaying actual rendering and affecting the user experience. The following Tracy screenshot shows a slow frame. Most of the frame is processing uploads of vertex buffers to the GPU:
Zoomed version of the same screenshot where we see actual rendering at the very end of the frame:
Pipelining the renderer would alleviate the problem but it won't be enough when many uploads are processed in the same frame. Also, pipelining the renderer requires significant efforts. An easier solution that may scale better is to distribute uploads to as many threads as possible and only wait for them to finish when they are needed in a draw call. This can scale better when many free CPU cores exist.
This PR adds shared EGL contexts to the Android backend to issue buffer uploads in worker threads.
The existing thread pool has been updated to enable persistent EGL contexts (I noticed very poor performance on Qualcomm drivers when migrating contexts between threads hence the thread pool with persistent contexts: no calls to
eglMakeCurrentevery frame and in different threads). This allows concurrent uploads to the GPU:The uploads are synchronized with the draws: the render thread only waits for a buffer when a draw needs it. This allow drawing while uploading data:
The uploads are currently randomly scheduled. Parallelizing drawing and uploading can be improved by first scheduling uploads for resources that get used late in the frame, e.g. uploading resources used by translucent objects before opaque objects since opaques are rendered first.
This PR is specific to Android and is made such as other backends are unaffected and have minimal code change. Once Vulkan is the official backend on Android similar and better handling of resources can be done.
Texture uploads are also slow but less frequent and will be handled in a separate PR:
There are unused index buffer that we currently wait on before we destroy them and this is slowing down the render thread #2760
We can destroy them in a future frame but we should understand why they are there in the first place.
There are slowdowns in the the render thread that will be investigated separately (mailbox related):
